fix: stop swallowing extraction errors in background task#173
Merged
Conversation
Remove the broad `except Exception` in ExtractorManager.__run_extractor. It only logged a one-line message and suppressed the exception, hiding the real traceback. Since extraction runs in a FastAPI BackgroundTask, the exception now propagates to Starlette and is logged with full traceback in the container logs, making failures diagnosable. The `finally` block still resets _active_extractor, so a crashed extraction no longer requires the except to avoid bricking the single-extractor lock.
Update test_run_extractor to reflect the new contract: __run_extractor no longer swallows exceptions. The test now asserts the exception propagates (pytest.raises) and that the finally block still resets _active_extractor, so a crashed extraction does not leave the single-extractor lock stuck.
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ExtractorManager.__run_extractorwrappedextractor.process()in a broadexcept Exceptionthat logged a single line ("Extraction failed with error") and swallowed the exception. The real traceback was lost.This actively hid a failure: when an extraction crashed,
_active_extractorwas reset toNone, so/v2/statusreported the service as idle/done even though nothing was produced. The e2e tests could only observe the symptom (No output files were created) with no way to see the underlying cause.Change
Remove the
exceptblock. Keepfinally.BackgroundTask, so the exception propagates to Starlette and is logged with a full traceback in the container logs.finallyblock still resets_active_extractor, so a crashed extraction does not brick the single-extractor lock (no permanent 409).started) is already sent before the background task runs, so there was never a 500 to lose.Result
Background-task failures are now diagnosable from container logs instead of being silently discarded.